-
Notifications
You must be signed in to change notification settings - Fork 436
feat(clerk-js): Improve getToken offline handling
#7598
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(clerk-js): Improve getToken offline handling
#7598
Conversation
🦋 Changeset detectedLatest commit: dfa5223 The changes in this PR will be included in the next version bump. This PR includes changesets to release 21 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
@bratsos I am concerned that this PR is going to conflict heavily with the stale-while-revalidate change: #7317 I think we will want to base this PR off feat/stale-while-revalidate-token and build on top of it. Some of the offline heuristics work well with SWR since we can return a cached token whenever possible and error only when we truly cannot fetch a new token |
b6350c6 to
640e362
Compare
getToken offline handlinggetToken offline handling
| '@clerk/tanstack-react-start': major | ||
| '@clerk/react-router': major | ||
| '@clerk/clerk-js': major | ||
| '@clerk/upgrade': major | ||
| '@clerk/nextjs': major | ||
| '@clerk/shared': major | ||
| '@clerk/react': major | ||
| '@clerk/nuxt': major | ||
| '@clerk/vue': major |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if this should be a major change in all sdks 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think so, but it doesn't really matter in practice as all of them are going to get a major anyways
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should I leave it like this then?
9de4869 to
c647c6a
Compare
jacekradko
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall. Just a couple of questions
BREAKING CHANGE: `getToken()` now throws `ClerkOfflineError` instead of returning `null` when the client is offline. This makes it explicit that the failure was due to network conditions, not authentication state.
c04b2a4 to
a36112f
Compare
📝 WalkthroughWalkthroughA new ClerkOfflineError class is added and re-exported from shared error modules. getToken() behavior changed: it now throws ClerkOfflineError when the client is offline instead of returning null. Session and core clerk logic were updated to catch and distinguish offline errors from other failures. Tests were added/updated to cover offline scenarios, and migration documentation describing the new try/catch usage was included. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Important Action Needed: IP Allowlist UpdateIf your organization protects your Git platform with IP whitelisting, please add the new CodeRabbit IP address to your allowlist:
Failure to add the new IP will result in interrupted reviews. Comment |
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
|
!allow-major |
Description
offline-demo.mp4
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
Breaking Changes
New Features
Documentation